fix(fs): follow final link under linked dirs; slash keys for implicit parents by chaliy · Pull Request #2841 · everruns/bashkit

chaliy · 2026-11-05T19:29:05Z

Requested by Михайло · project thread

What changed

Writes through a symlink in a symlinked directory replace the link (>) or fail (>>) #2829: writing to a link inside a symlinked directory (> dl/link with dl -> sub, sub/link -> ../file) now writes the link's target. Before, > silently replaced sub/link with a regular file and >> failed with "No such file or directory". The write path now resolves the final component even when the inner stat failed on a parent link, matching the FollowFs module contract ("writes never replace a link with a file").
On Windows, mkdir -p and add_file/add_dir key implicit parent directories with \ #2830: mkdir -p, InMemoryFs::add_file, add_dir and add_lazy_file build implicit parent keys with vfs_join instead of PathBuf::push, so snapshot() on Windows no longer exposes /tmp\a-style paths.

Thanks @CameronTofer for both reports and the suggested patches.

Why

Both were reported by a downstream user with repros on 0.20.3. #2829 silently corrupts the link graph; #2830 loses every mkdir -p directory for callers that map snapshot paths back.

Before / After

#2829 (new spec case symlink_write_through_link_in_linked_dir, expectation checked against real bash):

mkdir -p sub; printf a > file; ln -s ../file sub/link; ln -s sub dl
printf b >> dl/link; printf c >> dl/link; cat file   # before: error, a   after: abc
printf d > dl/link; cat file; readlink sub/link      # before: abc + link gone   after: d, ../file

#2830: new windows_containment_implicit_parents_use_slash_keys asserts no \ in entry keys; it runs in the existing Windows containment CI job (the bug is invisible on Unix hosts).

Local: cargo test -p bashkit --lib fs:: 224 → 226 pass (the #2829 unit test failed before the fix), bash spec suite green, clippy clean.

Risk

Low
Write-path resolution under linked dirs changes; only affects paths whose inner stat already failed.

Checklist

Tests added or updated
Backward compatibility considered

Closes #2829
Closes #2830

… parents - FollowFs write target resolves the final component when a parent is a link, so > replaces the target, not the link, and >> works (#2829) - InMemoryFs add_file/add_dir/add_lazy_file/mkdir -p build implicit parent keys with vfs_join, not PathBuf::push (#2830) Closes #2829 Closes #2830

bashkit-pr-reviewer

Correct root-cause fixes for both issues; no blocking problems.

bashkit-pr-reviewer · 2026-11-05T19:31:04Z

Review verdict

Acceptable — revised write_target is more precise than the previous push and keeps the fix; no blocking issues.

Findings (0 new)

P2 implicit-parent-slash-test-unix-vacuous from the last push is addressed: the author confirms the slash-key test runs in the Windows containment CI job and the bug is invisible on Unix hosts. No re-post.

What changed since last review (`318113a` → `5f3e14a`)

FollowFs::write_target Err branch no longer unconditionally resolve(path, true). It now does parent_followed first (returns the path untouched when no parent link, preserving the stat count for plain new-file writes), then stats the parent-resolved target and only resolve(target, true) when that final component is itself a symlink. Verified against crates/bashkit/src/fs/follow.rs:157-176 plus the unchanged test writes_through_link_in_linked_dir_follow_final_link: linked-dir + final-link (/dl/link → /sub/link → /file), new-file-under-linked-dir (/dl/new → /sub/new), and dangling-link-create-target cases all resolve correctly; ELOOP still propagates from either resolve.
No changes to memory.rs vfs_join fix, spec case, or knowledge entries beyond what was already reviewed.

Context read beyond the diff

crates/bashkit/src/fs/follow.rs 149–185, 410–455 (all write_target callers still write_file/append_file/copy-dest; lstat/remove/rename/symlink/mkdir still use parent_followed).

Not reviewed

Build/test not run (no cargo in this shell); reasoned from code per project rules.

…a parent link

chaliy self-assigned this Nov 5, 2026

bashkit-pr-reviewer Bot reviewed Nov 5, 2026

View reviewed changes

Comment thread crates/bashkit/src/fs/memory.rs Resolved

fix(fs): keep new-file write stat count; check final link only after …

5f3e14a

…a parent link

chaliy merged commit 2915b3b into main Nov 5, 2026
52 checks passed

chaliy deleted the claude/project-thread-jyp9jo branch November 5, 2026 19:59

Navigation Menu

Repository navigation

fix(fs): follow final link under linked dirs; slash keys for implicit parents - #2841

fix(fs): follow final link under linked dirs; slash keys for implicit parents#2841
chaliy merged 2 commits into
mainfrom
claude/project-thread-jyp9jo

chaliy commented Nov 5, 2026 •

edited

Loading

Uh oh!

bashkit-pr-reviewer Bot left a comment

Uh oh!

Uh oh!

bashkit-pr-reviewer Bot commented Nov 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

chaliy commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why

Before / After

Risk

Checklist

Uh oh!

bashkit-pr-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bashkit-pr-reviewer Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review verdict

Findings (0 new)

What changed since last review (318113a → 5f3e14a)

Context read beyond the diff

Not reviewed

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chaliy commented Nov 5, 2026 •

edited

Loading

bashkit-pr-reviewer Bot commented Nov 5, 2026 •

edited

Loading

What changed since last review (`318113a` → `5f3e14a`)